fix(skill-loader): discover skills from .agents/skills/ directory#1710
fix(skill-loader): discover skills from .agents/skills/ directory#1710dtateks wants to merge 2 commits intocode-yeongyu:devfrom
Conversation
Plugin's skill tool was overriding OpenCode's native SkillTool, but only loaded skills from .claude/skills/ and .opencode/skills/. Skills placed in .agents/skills/ (supported by OpenCode natively) were silently dropped. Add discoverProjectAgentsSkills() and discoverGlobalAgentsSkills() to match OpenCode's EXTERNAL_DIRS = [".claude", ".agents"] behavior.
|
Thank you for your contribution! Before we can merge this PR, we need you to sign our Contributor License Agreement (CLA). To sign the CLA, please comment on this PR with: This is a one-time requirement. Once signed, all your future contributions will be automatically accepted. I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
…code.skills flag .agents/skills/ is an OpenCode-native path, not a Claude Code compat feature. It should always be loaded like .opencode/skills/, not gated behind claude_code.skills config.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 files
Confidence score: 4/5
- This PR looks safe to merge overall, with only a low-severity test isolation concern.
mock.restore()insrc/features/opencode-skill-loader/loader.test.tsdoesn’t revertmock.module()in Bun, so a mockedos.homedir()can leak to later tests and cause intermittent failures.- Pay close attention to
src/features/opencode-skill-loader/loader.test.ts- ensure mocked module state doesn’t persist across tests.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/features/opencode-skill-loader/loader.test.ts">
<violation number="1" location="src/features/opencode-skill-loader/loader.test.ts:629">
P2: `mock.restore()` does not undo `mock.module()` overrides in Bun, so the mocked `os` module remains in the module cache after this test. This can leak the fake `homedir()` into subsequent tests and cause flakiness. Restore the module mock explicitly (e.g., re-mock `os` with the original exports) instead of relying on `mock.restore()`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| expect(skill?.scope).toBe("user") | ||
| expect(skill?.definition.description).toContain("A skill from global .agents/skills directory") | ||
| } finally { | ||
| mock.restore() |
There was a problem hiding this comment.
P2: mock.restore() does not undo mock.module() overrides in Bun, so the mocked os module remains in the module cache after this test. This can leak the fake homedir() into subsequent tests and cause flakiness. Restore the module mock explicitly (e.g., re-mock os with the original exports) instead of relying on mock.restore().
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/opencode-skill-loader/loader.test.ts, line 629:
<comment>`mock.restore()` does not undo `mock.module()` overrides in Bun, so the mocked `os` module remains in the module cache after this test. This can leak the fake `homedir()` into subsequent tests and cause flakiness. Restore the module mock explicitly (e.g., re-mock `os` with the original exports) instead of relying on `mock.restore()`.</comment>
<file context>
@@ -558,6 +559,120 @@ Skill body.
+ expect(skill?.scope).toBe("user")
+ expect(skill?.definition.description).toContain("A skill from global .agents/skills directory")
+ } finally {
+ mock.restore()
+ }
+ })
</file context>
|
Thank you for this contribution! The @dtateks is credited as Co-authored-by in the commit for the original idea and approach. Closing this in favor of #1834 which includes the expanded scope and additional test coverage. |
|
Superseded by #1834 |
2 participants
Summary
skilltool overrides OpenCode's nativeSkillTool, but was missing.agents/skills/directory support.agents/skills/(project-level) and~/.agents/skills/(global) were silently dropped when the plugin was activeEXTERNAL_DIRS = [".claude", ".agents"], but the plugin only covered.claude/skills/Root Cause
OpenCode's tool registry (
prompt.ts) builds tools astools[item.id] = .... Since plugin custom tools come after built-in tools, the plugin'sskilltool (id:"skill") overwrites OpenCode's nativeSkillTool. The plugin's skill discovery only searched.claude/skills/and.opencode/skills/, missing.agents/skills/entirely.Changes
loader.ts: AdddiscoverProjectAgentsSkills()(.agents/skills/, scope:"project") anddiscoverGlobalAgentsSkills()(~/.agents/skills/, scope:"user")skill-context.ts: Wire new discovery functions into the skill context pipelineloader.test.ts: Add 3 test cases covering project-level, global, and integration discoveryTesting
All 17 loader tests pass. No regressions.
Summary by cubic
Fixes skill discovery so the plugin always loads skills from .agents/skills at project and user scopes, matching OpenCode’s behavior. Prevents .agents skills from being dropped or gated behind the claude_code.skills flag.
Bug Fixes
Tests
Written for commit e4ed668. Summary will update on new commits.